-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[API Implementation]: SafeFileHandle overloads for SetCreationTime, SetLastAccessTime, SetLastWriteTime #60507
Conversation
Tagging subscribers to this area: @dotnet/area-system-io |
src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs
Outdated
Show resolved
Hide resolved
@danmoseley Do the files added under |
@deeprobin if you look at the build log you should see the project file on the end of the error:
You've introduced use of WIN32_FILE_ATTRIBUTE_DATA into a file in this project: the project file needs updating to include its source file. |
@deeprobin are you able to build the libraries locally successfully? An incremental build under src\libraries should be less than a minute, and ought to have thrown up these errors locally, which would be quicker for you than waiting an hour or whatever for the PR validation system. |
src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem/tests/File/GetSetAttributes.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs
Outdated
Show resolved
Hide resolved
Generally folks don't push changes one at a time, because it restarts PR validation, which is extensive -- 50+ configurations -- and that uses machine capacity. What I suggest is that you wait to push until you are satisfied with your changes and have done basic build and tests locally, then push everything (squashed or not as you prefer). Another option is that I mark this PR as draft, so that pushes don't kick off more validation (I think). What are your thoughts |
Unfortunately, VS support doesn't currently work for me with .NET 7, so I've always validated that via CI. |
You can build and run tests outside of VS. this is what I do. There is all the info you need in the workflow docs in this repo. |
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.BY_HANDLE_FILE_INFORMATION.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem/tests/File/GetSetAttributes.cs
Outdated
Show resolved
Hide resolved
@deeprobin it looks like there's unix specific build errors. You don't need to be on a Unix box to reproduce those. Something like this at the root of the repo: There should be a way to do it with dotnet build, but I have to check how. |
@danmoseley Thank you. I'll fixed it (the unix-build is now passing locally) |
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFileInformationByHandle.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj
Outdated
Show resolved
Hide resolved
src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs
Outdated
Show resolved
Hide resolved
It looks like you need to rebase on main as I made a concurrent change. It should be easy. |
Can you please open a new issue for your failure:
|
Cc @wfurt |
@jozkee @danmoseley |
@jozkee Can you give another review (or approve)? |
src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs
Show resolved
Hide resolved
@deeprobin if you are not planning on addressing the documentation feedback, could you please file an issue for it? |
I'll address it. Give me one hour ;) |
Co-Authored-By: David Cantú <[email protected]>
@jozkee Doc changes applied |
CI error is #66625 |
Thanks, @deeprobin. |
Proposal implementation of #20234 (closes #20234)
Proposal
Current state of implementation
/cc @Liryna